From c5e0280a061ba31f2926afd6e5d99380206ac743 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 29 Oct 2014 17:56:07 -0700 Subject: [PATCH] Add a custom join_paths function to wrap the error This helps us not unwrap() and rather propagate the error upwards. Closes #488 --- src/cargo/ops/cargo_run.rs | 3 ++- src/cargo/ops/cargo_rustc/compilation.rs | 28 ++++++++++++------------ src/cargo/ops/cargo_rustc/mod.rs | 19 ++++++++-------- src/cargo/ops/cargo_test.rs | 4 ++-- src/cargo/util/mod.rs | 2 +- src/cargo/util/paths.rs | 10 +++++++++ 6 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index bd1531491..b464c684f 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -48,7 +48,8 @@ pub fn run(manifest_path: &Path, Some(path) => path, None => exe, }; - let process = compile.process(exe, &root).args(args).cwd(os::getcwd()); + let process = try!(compile.process(exe, &root)) + .args(args).cwd(os::getcwd()); try!(options.shell.status("Running", process.to_string())); Ok(process.exec().err()) diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs index ebeeefe1f..646c79a38 100644 --- a/src/cargo/ops/cargo_rustc/compilation.rs +++ b/src/cargo/ops/cargo_rustc/compilation.rs @@ -1,10 +1,9 @@ use std::collections::HashMap; use std::dynamic_lib::DynamicLibrary; -use std::os; use semver::Version; use core::{PackageId, Package}; -use util; +use util::{mod, CargoResult}; /// A structure returning the result of a compilation. pub struct Compilation { @@ -60,30 +59,31 @@ impl Compilation { /// The package argument is also used to configure environment variables as /// well as the working directory of the child process. pub fn process(&self, cmd: T, pkg: &Package) - -> util::ProcessBuilder { + -> CargoResult { let mut search_path = DynamicLibrary::search_path(); for dir in self.native_dirs.values() { search_path.push(dir.clone()); } search_path.push(self.root_output.clone()); search_path.push(self.deps_output.clone()); - let search_path = os::join_paths(search_path.as_slice()).unwrap(); + let search_path = try!(util::join_paths(search_path.as_slice(), + DynamicLibrary::envvar())); let mut cmd = util::process(cmd).env(DynamicLibrary::envvar(), Some(search_path.as_slice())); for (k, v) in self.extra_env.iter() { cmd = cmd.env(k.as_slice(), v.as_ref().map(|s| s.as_slice())); } - cmd.env("CARGO_MANIFEST_DIR", Some(pkg.get_manifest_path().dir_path())) - .env("CARGO_PKG_VERSION_MAJOR", - Some(pkg.get_version().major.to_string())) - .env("CARGO_PKG_VERSION_MINOR", - Some(pkg.get_version().minor.to_string())) - .env("CARGO_PKG_VERSION_PATCH", - Some(pkg.get_version().patch.to_string())) - .env("CARGO_PKG_VERSION_PRE", - pre_version_component(pkg.get_version())) - .cwd(pkg.get_root()) + Ok(cmd.env("CARGO_MANIFEST_DIR", Some(pkg.get_manifest_path().dir_path())) + .env("CARGO_PKG_VERSION_MAJOR", + Some(pkg.get_version().major.to_string())) + .env("CARGO_PKG_VERSION_MINOR", + Some(pkg.get_version().minor.to_string())) + .env("CARGO_PKG_VERSION_PATCH", + Some(pkg.get_version().patch.to_string())) + .env("CARGO_PKG_VERSION_PRE", + pre_version_component(pkg.get_version())) + .cwd(pkg.get_root())) } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index ee023be19..34f6d1aa0 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -2,11 +2,10 @@ use std::collections::HashSet; use std::dynamic_lib::DynamicLibrary; use std::io::{fs, USER_RWX}; use std::io::fs::PathExtensions; -use std::os; use core::{SourceMap, Package, PackageId, PackageSet, Target, Resolve}; use util::{mod, CargoResult, ProcessBuilder, CargoError, human, caused_human}; -use util::{Require, Config, internal, ChainError, Fresh, profile}; +use util::{Require, Config, internal, ChainError, Fresh, profile, join_paths}; use self::job::{Job, Work}; use self::job_queue as jq; @@ -228,7 +227,7 @@ fn compile_custom(pkg: &Package, cmd: &str, let layout = cx.layout(pkg, KindTarget); let output = layout.native(pkg); let old_output = layout.proxy().old_native(pkg); - let mut p = process(cmd.next().unwrap(), pkg, cx) + let mut p = try!(process(cmd.next().unwrap(), pkg, cx)) .env("OUT_DIR", Some(&output)) .env("DEPS_DIR", Some(&output)) .env("TARGET", Some(cx.target_triple())) @@ -309,7 +308,7 @@ fn rustc(package: &Package, target: &Target, fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>, cx: &Context, req: PlatformRequirement) -> CargoResult> { - let base = process("rustc", package, cx); + let base = try!(process("rustc", package, cx)); let base = build_base_args(cx, base, package, target, crate_types.as_slice()); let target_cmd = build_plugin_args(base.clone(), cx, package, target, KindTarget); @@ -335,7 +334,7 @@ fn rustdoc(package: &Package, target: &Target, let kind = KindTarget; let pkg_root = package.get_root(); let cx_root = cx.layout(package, kind).proxy().dest().join("doc"); - let rustdoc = process("rustdoc", package, cx).cwd(pkg_root.clone()); + let rustdoc = try!(process("rustdoc", package, cx)).cwd(pkg_root.clone()); let mut rustdoc = rustdoc.arg(target.get_src_path()) .arg("-o").arg(cx_root) .arg("--crate-name").arg(target.get_name()); @@ -545,7 +544,8 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package, } } -pub fn process(cmd: T, pkg: &Package, cx: &Context) -> ProcessBuilder { +pub fn process(cmd: T, pkg: &Package, + cx: &Context) -> CargoResult { // When invoking a tool, we need the *host* deps directory in the dynamic // library search path for plugins and such which have dynamic dependencies. let layout = cx.layout(pkg, KindPlugin); @@ -567,9 +567,10 @@ pub fn process(cmd: T, pkg: &Package, cx: &Context) -> ProcessBuilder // We want to use the same environment and such as normal processes, but we // want to override the dylib search path with the one we just calculated. - let search_path = os::join_paths(search_path.as_slice()).unwrap(); - cx.compilation.process(cmd, pkg) - .env(DynamicLibrary::envvar(), Some(search_path.as_slice())) + let search_path = try!(join_paths(search_path.as_slice(), + DynamicLibrary::envvar())); + Ok(try!(cx.compilation.process(cmd, pkg)) + .env(DynamicLibrary::envvar(), Some(search_path.as_slice()))) } fn each_dep<'a>(pkg: &Package, cx: &'a Context, f: |&'a Package|) { diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 426fe9f80..3e8de8282 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -32,7 +32,7 @@ pub fn run_tests(manifest_path: &Path, Some(path) => path, None => exe.clone(), }; - let cmd = compile.process(exe, &compile.package).args(test_args); + let cmd = try!(compile.process(exe, &compile.package)).args(test_args); try!(options.compile_opts.shell.concise(|shell| { shell.status("Running", to_display.display().to_string()) })); @@ -58,7 +58,7 @@ pub fn run_tests(manifest_path: &Path, for (lib, name) in libs { try!(options.compile_opts.shell.status("Doc-tests", name)); - let mut p = compile.process("rustdoc", &compile.package) + let mut p = try!(compile.process("rustdoc", &compile.package)) .arg("--test").arg(lib) .arg("--crate-name").arg(name) .arg("-L").arg(&compile.root_output) diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 72fd9516f..e23ab7dc0 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -4,7 +4,7 @@ pub use self::result::{Wrap, Require}; pub use self::errors::{CargoResult, CargoError, BoxError, ChainError, CliResult}; pub use self::errors::{CliError, FromError, ProcessError}; pub use self::errors::{process_error, internal_error, internal, human, caused_human}; -pub use self::paths::realpath; +pub use self::paths::{realpath, join_paths}; pub use self::hex::{to_hex, short_hash}; pub use self::pool::TaskPool; pub use self::dependency_queue::{DependencyQueue, Fresh, Dirty, Freshness}; diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index bfdf0ca90..28441e4fd 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -1,5 +1,8 @@ use std::{io,os}; use std::io::fs; +use std::path::BytesContainer; + +use util::{human, CargoResult}; pub fn realpath(original: &Path) -> io::IoResult { static MAX_LINKS_FOLLOWED: uint = 256; @@ -38,3 +41,10 @@ pub fn realpath(original: &Path) -> io::IoResult { return Ok(result); } +pub fn join_paths(paths: &[T], env: &str) + -> CargoResult> { + os::join_paths(paths).map_err(|e| { + human(format!("failed to join search paths together: {}\n\ + Does ${} have an unterminated quote character?", e, env)) + }) +} -- 2.30.2